fix(lu): do not perf-warn on contiguous row-major input by shinaoka · Pull Request #347 · tensor4all/faer

shinaoka · 2026-11-09T13:50:51Z

Closes #345.

The defect

The perf-warn block of lu::partial_pivoting read

if (A.col_stride().unsigned_abs() == 1
	|| A.row_stride().unsigned_abs() != 1)
	&& crate::__perf_warn!(LU_WARN)

which is "not contiguous column-major": the first clause is true for a contiguous row-major input
(col_stride == 1) and the second for anything that is not column-major. So the message
"LU with partial pivoting prefers column-major or row-major matrix. Found matrix with generic
strides." fired on every contiguous row-major input, one of the two layouts it recommends. Only
column-major input was silent, because both clauses happen to be false there.

The change

has_generic_strides(row_stride, col_stride) = |row_stride| != 1 && |col_stride| != 1, the predicate
the message describes, used by the warning and unit-tested next to it: contiguous row-major,
contiguous column-major and a transposed view are silent, and a matrix with neither stride equal to one
warns. The two negative unit strides, (64, -1) and (-1, 64), carry the unsigned_abs: with the
helper body replaced by row_stride != 1 && col_stride != 1 every other case stays green and these two
fail (outputs below).

The predicate is only half of it: what the warning depends on is the call site, and the unit test
cannot see it — reverting lu_in_place to the old condition leaves all eight asserts green. So the
log is asserted from faer/tests/lu_perf_warn.rs, its own integration-test binary: a capturing
log::Log sees nothing for a contiguous row-major and a contiguous column-major input, exactly one
message for a row_stride = 2 view, and nothing more for a second such factorization. perf-warn is
enabled by no other lane in any workflow, so the build job of the ci workflow runs it as
cargo test -p t4a-faer --features perf-warn --test lu_perf_warn — one step in an existing job, no new
job and no new required check.

This is the same predicate as the full-pivoting fix in #342. The helper stays local to
partial_pivoting::factor until both land; then it belongs in lu/mod.rs with one copy, and #342's
copy can be dropped in the same rebase.

No arithmetic path changes: the gated block is the log::warn! and nothing else.

Verification

Red: the new test against the old predicate, moved into the helper verbatim

The predicate was first transcribed into the helper with its old body
(col_stride.unsigned_abs() == 1 || row_stride.unsigned_abs() != 1), the test written, and the test
run. It fails on the row-major case, which is the reported defect:

$ cargo test -p t4a-faer --lib lu::partial_pivoting::factor::tests::perf_warn_predicate_excludes_both_preferred_layouts
warning: unused imports: `apply_block_householder_on_the_left_in_place_scratch` and `apply_block_householder_on_the_left_in_place_with_conj`
    --> faer/src/linalg/householder.rs:1082:3
     |
1082 |         apply_block_householder_on_the_left_in_place_scratch,
     |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1083 |         apply_block_householder_on_the_left_in_place_with_conj,
     |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     |
     = note: `#[warn(unused_imports)]` (part of `#[warn(unused)]`) on by default

warning: unused import: `crate::internal_prelude::*`
   --> faer/src/linalg/lq/no_pivoting/factor.rs:113:6
    |
113 |     use crate::internal_prelude::*;
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: unused import: `std::num::NonZeroUsize`
    --> faer/src/linalg/matmul/mod.rs:2085:6
     |
2085 |     use std::num::NonZeroUsize;
     |         ^^^^^^^^^^^^^^^^^^^^^^

warning: unused variable: `c0`
   --> faer/src/linalg/lq/no_pivoting/factor.rs:182:14
    |
182 |                 let (p0, c0, r0) = run(0, &mut 0, false);
    |                          ^^ help: if this is intentional, prefix it with an underscore: `_c0`
    |
    = note: `#[warn(unused_variables)]` (part of `#[warn(unused)]`) on by default

warning: `t4a-faer` (lib test) generated 4 warnings (run `cargo fix --lib -p t4a-faer --tests` to apply 4 suggestions)
    Finished `test` profile [optimized + debuginfo] target(s) in 0.60s
     Running unittests src/lib.rs (target/debug/deps/faer-a14509338449283a)

running 1 test
test linalg::lu::partial_pivoting::factor::tests::perf_warn_predicate_excludes_both_preferred_layouts ... FAILED

failures:

---- linalg::lu::partial_pivoting::factor::tests::perf_warn_predicate_excludes_both_preferred_layouts stdout ----

thread 'linalg::lu::partial_pivoting::factor::tests::perf_warn_predicate_excludes_both_preferred_layouts' (3012112) panicked at faer/src/linalg/lu/partial_pivoting/factor.rs:762:9:
Assertion failed at faer/src/linalg/lu/partial_pivoting/factor.rs:762:3
Assertion failed: ! has_generic_strides(64, 1)
- ! has_generic_strides(64, 1) = false
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    linalg::lu::partial_pivoting::factor::tests::perf_warn_predicate_excludes_both_preferred_layouts

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 861 filtered out; finished in 0.00s

error: test failed, to rerun pass `-p t4a-faer --lib`

Green: the same test file, helper body fixed

$ cargo test -p t4a-faer --lib lu::partial_pivoting::factor::tests::perf_warn_predicate_excludes_both_preferred_layouts
   Compiling t4a-faer v0.32.0 (/home/shinaoka/projects/tensor4all/faer/faer)
warning: unused imports: `apply_block_householder_on_the_left_in_place_scratch` and `apply_block_householder_on_the_left_in_place_with_conj`
    --> faer/src/linalg/householder.rs:1082:3
     |
1082 |         apply_block_householder_on_the_left_in_place_scratch,
     |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1083 |         apply_block_householder_on_the_left_in_place_with_conj,
     |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     |
     = note: `#[warn(unused_imports)]` (part of `#[warn(unused)]`) on by default

warning: unused import: `crate::internal_prelude::*`
   --> faer/src/linalg/lq/no_pivoting/factor.rs:113:6
    |
113 |     use crate::internal_prelude::*;
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: unused import: `std::num::NonZeroUsize`
    --> faer/src/linalg/matmul/mod.rs:2085:6
     |
2085 |     use std::num::NonZeroUsize;
     |         ^^^^^^^^^^^^^^^^^^^^^^

warning: unused variable: `c0`
   --> faer/src/linalg/lq/no_pivoting/factor.rs:182:14
    |
182 |                 let (p0, c0, r0) = run(0, &mut 0, false);
    |                          ^^ help: if this is intentional, prefix it with an underscore: `_c0`
    |
    = note: `#[warn(unused_variables)]` (part of `#[warn(unused)]`) on by default

warning: `t4a-faer` (lib test) generated 4 warnings (run `cargo fix --lib -p t4a-faer --tests` to apply 4 suggestions)
    Finished `test` profile [optimized + debuginfo] target(s) in 8m 36s
     Running unittests src/lib.rs (target/debug/deps/faer-a14509338449283a)

running 1 test
test linalg::lu::partial_pivoting::factor::tests::perf_warn_predicate_excludes_both_preferred_layouts ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 861 filtered out; finished in 0.00s

Red: the `unsigned_abs` mutant

Helper body replaced by row_stride != 1 && col_stride != 1, the mutant the review showed the old six
cases cannot catch:

$ cargo test -p t4a-faer --lib lu::partial_pivoting::factor::tests::perf_warn_predicate_excludes_both_preferred_layouts
   Compiling t4a-faer v0.32.0 (/home/shinaoka/projects/tensor4all/faer/faer)
warning: unused imports: `apply_block_householder_on_the_left_in_place_scratch` and `apply_block_householder_on_the_left_in_place_with_conj`
    --> faer/src/linalg/householder.rs:1082:3
     |
1082 |         apply_block_householder_on_the_left_in_place_scratch,
     |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1083 |         apply_block_householder_on_the_left_in_place_with_conj,
     |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     |
     = note: `#[warn(unused_imports)]` (part of `#[warn(unused)]`) on by default

warning: unused import: `crate::internal_prelude::*`
   --> faer/src/linalg/lq/no_pivoting/factor.rs:113:6
    |
113 |     use crate::internal_prelude::*;
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: unused import: `std::num::NonZeroUsize`
    --> faer/src/linalg/matmul/mod.rs:2085:6
     |
2085 |     use std::num::NonZeroUsize;
     |         ^^^^^^^^^^^^^^^^^^^^^^

warning: unused variable: `c0`
   --> faer/src/linalg/lq/no_pivoting/factor.rs:182:14
    |
182 |                 let (p0, c0, r0) = run(0, &mut 0, false);
    |                          ^^ help: if this is intentional, prefix it with an underscore: `_c0`
    |
    = note: `#[warn(unused_variables)]` (part of `#[warn(unused)]`) on by default

warning: `t4a-faer` (lib test) generated 4 warnings (run `cargo fix --lib -p t4a-faer --tests` to apply 4 suggestions)
    Finished `test` profile [optimized + debuginfo] target(s) in 11.65s
     Running unittests src/lib.rs (target/debug/deps/faer-a14509338449283a)

running 1 test
test linalg::lu::partial_pivoting::factor::tests::perf_warn_predicate_excludes_both_preferred_layouts ... FAILED

failures:

---- linalg::lu::partial_pivoting::factor::tests::perf_warn_predicate_excludes_both_preferred_layouts stdout ----

thread 'linalg::lu::partial_pivoting::factor::tests::perf_warn_predicate_excludes_both_preferred_layouts' (3067053) panicked at faer/src/linalg/lu/partial_pivoting/factor.rs:770:9:
Assertion failed at faer/src/linalg/lu/partial_pivoting/factor.rs:770:3
Assertion failed: ! has_generic_strides(64, -1)
- ! has_generic_strides(64, -1) = false
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    linalg::lu::partial_pivoting::factor::tests::perf_warn_predicate_excludes_both_preferred_layouts

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 861 filtered out; finished in 0.00s

error: test failed, to rerun pass `-p t4a-faer --lib`

Green: the same test with the helper restored

The mutant was reverted before this run; the tree is otherwise identical.

$ cargo test -p t4a-faer --lib lu::partial_pivoting::factor::tests::perf_warn_predicate_excludes_both_preferred_layouts
   Compiling t4a-faer v0.32.0 (/home/shinaoka/projects/tensor4all/faer/faer)
warning: unused imports: `apply_block_householder_on_the_left_in_place_scratch` and `apply_block_householder_on_the_left_in_place_with_conj`
    --> faer/src/linalg/householder.rs:1082:3
     |
1082 |         apply_block_householder_on_the_left_in_place_scratch,
     |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1083 |         apply_block_householder_on_the_left_in_place_with_conj,
     |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     |
     = note: `#[warn(unused_imports)]` (part of `#[warn(unused)]`) on by default

warning: unused import: `crate::internal_prelude::*`
   --> faer/src/linalg/lq/no_pivoting/factor.rs:113:6
    |
113 |     use crate::internal_prelude::*;
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: unused import: `std::num::NonZeroUsize`
    --> faer/src/linalg/matmul/mod.rs:2085:6
     |
2085 |     use std::num::NonZeroUsize;
     |         ^^^^^^^^^^^^^^^^^^^^^^

warning: unused variable: `c0`
   --> faer/src/linalg/lq/no_pivoting/factor.rs:182:14
    |
182 |                 let (p0, c0, r0) = run(0, &mut 0, false);
    |                          ^^ help: if this is intentional, prefix it with an underscore: `_c0`
    |
    = note: `#[warn(unused_variables)]` (part of `#[warn(unused)]`) on by default

warning: `t4a-faer` (lib test) generated 4 warnings (run `cargo fix --lib -p t4a-faer --tests` to apply 4 suggestions)
    Finished `test` profile [optimized + debuginfo] target(s) in 8.20s
     Running unittests src/lib.rs (target/debug/deps/faer-a14509338449283a)

running 1 test
test linalg::lu::partial_pivoting::factor::tests::perf_warn_predicate_excludes_both_preferred_layouts ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 861 filtered out; finished in 0.00s

Red: the call site, with the predicate untouched

lu_in_place's guard put back to the old condition
(A.col_stride().unsigned_abs() == 1 || A.row_stride().unsigned_abs() != 1); the helper and its unit
test are unchanged. The predicate test still passes under this mutant, which is the gap the review
found:

$ cargo test -p t4a-faer --lib lu::partial_pivoting::factor::tests::perf_warn_predicate_excludes_both_preferred_layouts
test linalg::lu::partial_pivoting::factor::tests::perf_warn_predicate_excludes_both_preferred_layouts ... ok
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 861 filtered out; finished in 0.00s

The integration test drives lu_in_place and reads the log, so it fails:

$ cargo test -p t4a-faer --features perf-warn --test lu_perf_warn
     Running tests/lu_perf_warn.rs (target/debug/deps/lu_perf_warn-255fed2cab666b59)

running 1 test
test the_warning_fires_once_and_only_for_generic_strides ... FAILED

thread 'the_warning_fires_once_and_only_for_generic_strides' panicked at faer/tests/lu_perf_warn.rs:96:5:
assertion `left == right` failed: contiguous row-major must stay silent
  left: 1
 right: 0

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
error: test failed, to rerun pass `-p t4a-faer --test lu_perf_warn`

Green: the call site restored

$ cargo test -p t4a-faer --features perf-warn --test lu_perf_warn
     Running tests/lu_perf_warn.rs (target/debug/deps/lu_perf_warn-255fed2cab666b59)

running 1 test
test the_warning_fires_once_and_only_for_generic_strides ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

`cargo test --lib lu::partial_pivoting`, full output

$ cargo test -p t4a-faer --lib lu::partial_pivoting
   Compiling t4a-faer v0.32.0 (/home/shinaoka/projects/tensor4all/faer/faer)
warning: unused imports: `apply_block_householder_on_the_left_in_place_scratch` and `apply_block_householder_on_the_left_in_place_with_conj`
    --> faer/src/linalg/householder.rs:1082:3
     |
1082 |         apply_block_householder_on_the_left_in_place_scratch,
     |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1083 |         apply_block_householder_on_the_left_in_place_with_conj,
     |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     |
     = note: `#[warn(unused_imports)]` (part of `#[warn(unused)]`) on by default

warning: unused import: `crate::internal_prelude::*`
   --> faer/src/linalg/lq/no_pivoting/factor.rs:113:6
    |
113 |     use crate::internal_prelude::*;
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: unused import: `std::num::NonZeroUsize`
    --> faer/src/linalg/matmul/mod.rs:2085:6
     |
2085 |     use std::num::NonZeroUsize;
     |         ^^^^^^^^^^^^^^^^^^^^^^

warning: unused variable: `c0`
   --> faer/src/linalg/lq/no_pivoting/factor.rs:182:14
    |
182 |                 let (p0, c0, r0) = run(0, &mut 0, false);
    |                          ^^ help: if this is intentional, prefix it with an underscore: `_c0`
    |
    = note: `#[warn(unused_variables)]` (part of `#[warn(unused)]`) on by default

warning: `t4a-faer` (lib test) generated 4 warnings (run `cargo fix --lib -p t4a-faer --tests` to apply 4 suggestions)
    Finished `test` profile [optimized + debuginfo] target(s) in 41.25s
     Running unittests src/lib.rs (target/debug/deps/faer-a14509338449283a)

running 12 tests
test linalg::lu::partial_pivoting::factor::tests::perf_warn_predicate_excludes_both_preferred_layouts ... ok
test linalg::lu::partial_pivoting::factor::tests::region_rules_follow_the_serial_time ... ok
test linalg::lu::partial_pivoting::factor::tests::simd_leaf_picks_the_scalar_pivots ... ok
test linalg::lu::partial_pivoting::factor::tests::test_plu ... ok
test linalg::lu::partial_pivoting::factor::tests::derived_gates_keep_the_factorization ... ok
test linalg::lu::partial_pivoting::factor::tests::f64_default_leaf_factors ... ok
test linalg::lu::partial_pivoting::factor::tests::threaded_lu_matches_the_sequential_one ... ok
test linalg::lu::partial_pivoting::inverse::tests::test_inverse ... ok
test linalg::lu::partial_pivoting::reconstruct::tests::test_reconstruct ... ok
test linalg::lu::partial_pivoting::factor::tests::wide_matrix_parallel_agrees_with_serial ... ok
test linalg::lu::partial_pivoting::solve::tests::test_solve ... ok
test linalg::lu::partial_pivoting::factor::tests::zero_pivot_column_reconstructs ... ok

test result: ok. 12 passed; 0 failed; 0 ignored; 0 measured; 850 filtered out; finished in 6.22s

The same filter with the feature that compiles the changed block:

$ cargo test -p t4a-faer --lib --features perf-warn lu::partial_pivoting
running 12 tests
test linalg::lu::partial_pivoting::factor::tests::f64_default_leaf_factors ... ok
test linalg::lu::partial_pivoting::factor::tests::perf_warn_predicate_excludes_both_preferred_layouts ... ok
test linalg::lu::partial_pivoting::factor::tests::derived_gates_keep_the_factorization ... ok
test linalg::lu::partial_pivoting::factor::tests::simd_leaf_picks_the_scalar_pivots ... ok
test linalg::lu::partial_pivoting::factor::tests::region_rules_follow_the_serial_time ... ok
test linalg::lu::partial_pivoting::factor::tests::test_plu ... ok
test linalg::lu::partial_pivoting::factor::tests::wide_matrix_parallel_agrees_with_serial ... ok
test linalg::lu::partial_pivoting::factor::tests::threaded_lu_matches_the_sequential_one ... ok
test linalg::lu::partial_pivoting::inverse::tests::test_inverse ... ok
test linalg::lu::partial_pivoting::reconstruct::tests::test_reconstruct ... ok
test linalg::lu::partial_pivoting::factor::tests::zero_pivot_column_reconstructs ... ok
test linalg::lu::partial_pivoting::solve::tests::test_solve ... ok
test result: ok. 12 passed; 0 failed; 0 ignored; 0 measured; 850 filtered out; finished in 12.15s

The crate's own suite, and a build of the configuration CI builds first:

$ cargo test -p t4a-faer --lib
running 862 tests
test result: ok. 700 passed; 0 failed; 162 ignored; 0 measured; 0 filtered out; finished in 154.98s

$ cargo check -p t4a-faer --no-default-features --features "std,linalg"
    Finished `dev` profile [optimized + debuginfo] target(s) in 36.55s      # 24 pre-existing warnings, none about this change

cargo check matters here because the helper is #[cfg(any(feature = "perf-warn", test))]: a plain
non-test build must not see an unused or dead function, and it does not.

Formatting

Both checks are red on this host at the parent revision too; the tree is not format-clean under the
installed rustfmt, because rustfmt.toml uses unstable options that stable ignores. The measurement
below is therefore a before/after count, not a pass (rustfmt 1.9.0-stable):

$ git show HEAD:faer/src/linalg/lu/partial_pivoting/factor.rs > faer/src/linalg/lu/partial_pivoting/factor.rs
$ cargo fmt --check 2>/dev/null | grep -c '^Diff in'
706                                    # parent revision
$ <restore this change>
$ cargo fmt --check 2>/dev/null | grep -c '^Diff in'
706                                    # with this change
$ cargo fmt --check 2>/dev/null | grep -o 'lu/partial_pivoting/factor.rs:[0-9]*'
27 56 63 72 79 89 102 110 117 146 204 287 316 323 342 349 354 410 474 735 754 777 806 830 838 845 870 964 994 1006 1053 1146 1179 1188 1200 1207

None of those hunks is on a line this change adds or rewrites (it touches 627-630 and 767-768). CI runs
this step with continue-on-error: true.

Clippy

-D warnings is also red at the parent revision, and code-quality.yml states why the crate is not a
clippy gate: "t4a-faer: 2340 warnings on 1.96.0, measured 2026-11-07. Not a gate". What matters here
is that no finding is on an added line:

$ cargo clippy -p t4a-faer --all-targets -- -D warnings 2>&1 | grep -c '^error'
2646                                   # parent revision, clippy 0.1.96
2646                                   # with this change: identical, nothing was added
$ ... | grep -o 'lu/partial_pivoting/factor.rs:[0-9]*' | sed 's/.*://' | sort -n -u
176 177 232 246 255 256 435 699 716     # parent revision
176 177 232 246 255 256 435 698 715     # with this change; the shift is the doc line removed

Upstream

faer 0.24.4 and upstream main at 0ecd846 (2026-10-22) carry the same predicate, unchanged:

src/linalg/lu/partial_pivoting/factor.rs:245-253   (faer 0.24.4; the same lines at 0ecd846)
	if (A.col_stride().unsigned_abs() == 1
		|| A.row_stride().unsigned_abs() != 1)
		&& crate::__perf_warn!(LU_WARN)

So this is an inherited defect and not a fork divergence, and a report is added rather than staying in
the commit message: upstream-reports/faer-042-lu-perf-warn-row-major reproduces it against the
published 0.24.4 crate with a counting log::Log, exits 1 on a contiguous row-major 64 x 64 input and
0 on the -- column-major control:

$ cd upstream-reports/faer-042-lu-perf-warn-row-major && cargo run --locked
warning: LU with partial pivoting prefers column-major or row-major matrix. Found matrix with generic strides.
input: 64 x 64 contiguous row-major (row_stride = 64, col_stride = 1)
factorisation: partial pivoting, 0 transpositions
expected `faer_perf` warnings: 0
actual `faer_perf` warnings: 1
exit 1

$ cargo run --locked -- column-major
input: 64 x 64 contiguous column-major (row_stride = 1, col_stride = 64)
factorisation: partial pivoting, 0 transpositions
expected `faer_perf` warnings: 0
actual `faer_perf` warnings: 0
exit 0

faer-042 depends on the published crate (faer = "=0.24.4" in its Cargo.toml), so it reports the
upstream defect and does not see the fork's wiring: it is not the regression test for this PR. The
fork's coverage is faer/tests/lu_perf_warn.rs, run by the build job of the ci workflow (step
cargo test -p t4a-faer --features perf-warn --test lu_perf_warn).

Execution scope: the two faer-042 invocations above (the row-major case and the -- column-major
control) were re-run against this branch on 2026-11-10. The other 43 report programs are unrelated to
this change and were not re-run here; the upstream-reports/README.md line saying every one of the 44
has been executed is that file's own record from 2026-11-09, not a claim of this PR.

No upstream issue or PR was prepared: AGENTS.md defers upstream feedback and leaves that decision to
the maintainer. The full-pivoting sibling (src/linalg/lu/full_pivoting/factor.rs:477-485 upstream) is
the same predicate in another file and is noted inside faer-042 rather than duplicated as its own
entry; the fork fixes that copy in #342.

Not measured

The acceptance grid is not run: the change reaches no arithmetic path, so there is no cell whose
timing it could move. The only code that differs is the condition guarding a log::warn!.

CI

Head 67f791c1, all checks green: body, build, code-quality, testing-stable-ubuntu-latest,
msrv-1.89.0-ubuntu-latest, msrv-1.89.0-windows-latest,
nostd-nightly-2025-08-07-ubuntu-latest, nostd-nightly-2025-08-07-windows-latest.

Head 1c880f5c (this head): the same checks, all green, and the new lane did run there — the step
output from the build job:

$ # .github/workflows/ci.yml, job `build`: cargo test -p t4a-faer --features perf-warn --test lu_perf_warn
running 1 test
test the_warning_fires_once_and_only_for_generic_strides ... ok
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

Branch 345-lu-partial-perf-warn-row-major, head 1c880f5c: 99580691 (the predicate fix),
67f791c1 (negative unit strides in the predicate test, unmeasured claim dropped), 5a192e97 (the
call-site test) and 1c880f5c (its CI lane). This is PR #347.

The `perf-warn` predicate of the column-pivoted factorisation read `col_stride == 1 || row_stride != 1`, so every contiguous row-major input warned "Found matrix with generic strides" -- a layout the message itself names as preferred. Only a contiguous column-major input was silent, because both clauses happen to be false there. Use the predicate the message describes, `has_generic_strides(row_stride, col_stride) = |row_stride| != 1 && |col_stride| != 1`, and unit-test the layouts next to it: contiguous row-major, contiguous column-major and a transposed view are silent, a matrix with neither stride equal to one warns. That is the same predicate as the full-pivoting fix in #342; the helper stays local to this module until both land, then it can move to `lu/mod.rs`. Only the message is affected: no arithmetic path changes, so there is no measurement to make. Upstream `faer` 0.24.4 and its `main` at `0ecd846` carry the same predicate (`faer/src/linalg/lu/partial_pivoting/factor.rs:245-253`). Recorded as `upstream-reports/faer-042-lu-perf-warn-row-major`; nothing was filed upstream, per AGENTS.md's deferred-feedback rule.

shinaoka · 2026-11-09T14:22:09Z

GPT-6.1-sol adversarial review

CRITICAL: none
NEEDS_CHANGES: 1
UNVERIFIED: 2

Critical (must not merge)

none

Needs changes

Negative unit strides are not regression-tested — faer/src/linalg/lu/partial_pivoting/factor.rs:762–773: replace the helper body with row_stride != 1 && col_stride != 1; all six supplied assertions still pass, although (64, -1) and (-1, 64) now incorrectly trigger the warning. I verified this mutation against the six supplied input/output cases using Python, not Cargo. The existing negative cases therefore do not establish the PR’s claim that “unsigned_abs is checked too.” Add assertions rejecting both negative-unit-stride cases.

Unverified claims

Equal performance of the preferred layouts — faer/src/linalg/lu/partial_pivoting/factor.rs:629–630: “Both preferred layouts factor at the same speed” is not established by this diff, and the PR explicitly says the acceptance grid was not run. Reproduction requires timing identical matrices in both layouts with the same effective thread count and configuration; no such measurements are supplied. Remove the speed assertion unless supporting measurements exist.
Verification of the submitted revision and CI behavior — faer/src/linalg/lu/partial_pivoting/factor.rs:649–652: the supplied transcripts report local results but do not establish successful checks on head 99580691f7583e5d5b85cb36407f22463a64f63e; I cannot read the actual check runs, workflow definitions, or execution artifacts from the supplied diff. Verify that revision’s check records and feature configuration before treating CI as evidence that the perf-warn block was exercised; the reported full-suite result also explicitly leaves 162 tests ignored.

Checked and fine

none

Not checked: repository code outside the supplied diff, Cargo execution, or live CI; runtime and performance claims remain unverified.

Reviewed head: 99580691f7583e5d5b85cb36407f22463a64f63e at 2026-11-09T14:21:39Z. This review is valid only for that commit: if the head has moved, it is a review of a tree that is not being merged.
Automated (gpt_review.sh). This is an enumeration of counterexamples, not a verdict; the merge decision stays with a human.

`has_generic_strides` reads `|row_stride| != 1 && |col_stride| != 1`, but replacing its body with `row_stride != 1 && col_stride != 1` kept all six cases added with it green: none is a stride of magnitude one on the axis the other one walks, so `unsigned_abs` was untested. Add `(64, -1)` and `(-1, 64)`; with the mutant the first fails (`! has_generic_strides(64, -1) = false`), with the real helper both pass. Drop the unmeasured claim that both preferred layouts factor at the same speed from the helper's doc comment.

shinaoka · 2026-11-09T15:08:36Z

GPT-6.1-sol adversarial review

CRITICAL: none
NEEDS_CHANGES: 1
UNVERIFIED: 2

Critical (must not merge)

none

Needs changes

The regression test does not exercise the warning being fixed — faer/src/linalg/lu/partial_pivoting/factor.rs:651-652, 764-773: restore the old condition at the lu_in_place call site while leaving has_generic_strides unchanged, then run cargo test -p t4a-faer --lib --features perf-warn lu::partial_pivoting::factor::tests::perf_warn_predicate_excludes_both_preferred_layouts; all eight assertions still pass even though row-major factorization warns again. The added executable cannot cover this wiring regression because upstream-reports/faer-042-lu-perf-warn-row-major/Cargo.toml:12 selects published upstream faer, not the modified fork. Add a fork-side logging regression test that invokes lu_in_place and checks the warning, with process isolation for the warn-once flag.

Unverified claims

The cited green CI belongs to a different commit — faer/src/linalg/lu/partial_pivoting/factor.rs:770-771 contains the follow-up negative-unit-stride assertions, but the PR body cites checks for 99580691 while the supplied head is 67f791c18eb0939983f824d8c3a967af1f8fd1c3. Reproduce the evidence check by comparing each check run’s head_sha with the supplied head; checks attached only to 99580691 cannot establish validation of the submitted state. Current check records and workflow step logs: cannot read from the supplied diff; actual execution versus skipped lanes remains unverified.
The historical execution and upstream-version claims are not established by the patch — upstream-reports/README.md:67-68 asserts all 44 programs were executed, and upstream-reports/faer-042-lu-perf-warn-row-major/README.md:5, 18-21 asserts behavior on published 0.24.4 and upstream 0ecd846. The supplied patch contains neither execution artifacts for the 44-program sweep nor the upstream checkout. To verify, run the documented sweeps and both reproducer modes, then inspect and test 0ecd846 separately. The pasted output establishes neither an independently reproduced run nor a run against that upstream commit; those artifacts cannot read from the supplied diff.

Checked and fine

none

Not checked: compilation, runtime logging, live CI, or source outside the supplied diff; the test-wiring finding is a source-level mutation analysis, not an executed mutation.

Reviewed head: 67f791c18eb0939983f824d8c3a967af1f8fd1c3 at 2026-11-09T15:07:48Z. This review is valid only for that commit: if the head has moved, it is a review of a tree that is not being merged.
Automated (gpt_review.sh). This is an enumeration of counterexamples, not a verdict; the merge decision stays with a human.

…345) The predicate test cannot see the wiring: restoring the old "not column-major" condition at the lu_in_place call site leaves it green. A unit test inside the crate cannot own the warn-once flag either, so add faer/tests/lu_perf_warn.rs, its own binary, with a capturing logger: contiguous row-major and column-major stay silent, a matrix with neither stride equal to one warns once, and a second such factor adds nothing. log is a dev-dependency so the test can install the logger.

shinaoka · 2026-11-09T15:31:45Z

GPT-6.1-sol adversarial review

CRITICAL: none
NEEDS_CHANGES: 1
UNVERIFIED: 3

Critical (must not merge)

none

Needs changes

The call-site regression test is disabled in CI — faer/tests/lu_perf_warn.rs:1, .github/workflows/run-tests.yml:124, and faer/Cargo.toml:114,122: the integration test requires perf-warn, but the test job enables only unstable, which does not enable it. Reproduce by restoring the old condition only at the call site in faer/src/linalg/lu/partial_pivoting/factor.rs:651, leaving the helper unchanged, then running the workflow’s test command: the predicate unit test cannot detect that mutation and the integration test is excluded. Add a CI invocation of cargo test -p t4a-faer --features perf-warn --test lu_perf_warn; the PR body’s --lib commands do not execute this integration test either.

Unverified claims

The listed green CI does not establish the submitted head’s status — faer/tests/lu_perf_warn.rs:1–135: the body explicitly cites 99580691, whereas the supplied head is 5a192e9708db026c853881810dcd8c8bbee452a9. Checking PR fix(lu): do not perf-warn on contiguous row-major input #347 returned build and testing-stable-ubuntu-latest as in progress, not successful. Reproduce by inspecting the checks for the submitted head; the older green list is not final-head evidence.
The upstream reproduction’s recorded execution is not established by its checked-in source — upstream-reports/faer-042-lu-perf-warn-row-major/Cargo.toml:11–12 and upstream-reports/README.md:67–68: the manifest comment says the local run used a checkout, while the body asserts execution against the published, locked dependency and the index asserts that all 44 programs were executed. Reproduce with cargo run --locked and cargo run --locked -- column-major from the report directory, recording the resolved dependency and exit statuses. Those executions were not independently rerun here; the diff cannot establish the historical execution claims.
“The factorisation itself is correct” is not checked by the reproducer — upstream-reports/faer-042-lu-perf-warn-row-major/README.md:3 and upstream-reports/faer-042-lu-perf-warn-row-major/src/main.rs:62–79: the program prints the transposition count and checks only the warning count, without reconstructing the input or checking a residual. Reproduce the verification gap by corrupting a factor entry after lu_in_place: the column-major control still exits successfully. Remove the correctness assertion or supply a numerical check.

Checked and fine

none

Not checked: local compilation, test execution, mutation runs, or the final completion of pending CI. Review evidence is limited to the supplied diff and retrieved head files/check statuses.

Reviewed head: 5a192e9708db026c853881810dcd8c8bbee452a9 at 2026-11-09T15:30:51Z. This review is valid only for that commit: if the head has moved, it is a review of a tree that is not being merged.
Automated (gpt_review.sh). This is an enumeration of counterexamples, not a verdict; the merge decision stays with a human.

…esult The integration test only compiles under perf-warn, which the test job does not enable, so the call-site regression would never run in CI. Add that invocation. The upstream report checks warning counts only, so its README no longer says the factorisation is correct.

shinaoka · 2026-11-09T15:35:08Z

GPT-6.1-sol adversarial review

CRITICAL: none
NEEDS_CHANGES: 0
UNVERIFIED: 2

Critical (must not merge)

none

Needs changes

none

Unverified claims

.github/workflows/ci.yml:31 — The reported green CI belongs to 99580691, not the supplied head 1c880f5ca5f49a06e8150d67769532515ff8b957. The pasted test logs also do not show execution of the newly added lu_perf_warn integration binary. To verify the submitted state, inspect check-run head_sha values and the output of cargo test -p t4a-faer --features perf-warn --test lu_perf_warn at 1c880f5c; cannot read those run records from the supplied material.
upstream-reports/README.md:67–68 — “Every one” of the 44 programs was executed on the stated date is not established by the diff. The count is consistent with the documented groups (39 + 3 + 1 + 1), but counting entries does not establish execution, dependency provenance, or exit status. Verify the dated execution artifacts for the listed sweep commands and both new reproducer invocations; cannot read those artifacts from the supplied material.

Checked and fine

none

Not checked: compilation, runtime mutation tests, or source outside the supplied diff; remaining risk is unverified execution evidence for the submitted head.

Reviewed head: 1c880f5ca5f49a06e8150d67769532515ff8b957 at 2026-11-09T15:34:36Z. This review is valid only for that commit: if the head has moved, it is a review of a tree that is not being merged.
Automated (gpt_review.sh). This is an enumeration of counterexamples, not a verdict; the merge decision stays with a human.

shinaoka requested a review from terasakisatoshi as a code owner November 9, 2026 13:50

shinaoka self-assigned this Nov 9, 2026

shinaoka merged commit 1be2ace into main Nov 9, 2026
8 checks passed

shinaoka deleted the 345-lu-partial-perf-warn-row-major branch November 9, 2026 15:58

Navigation Menu

Repository navigation

fix(lu): do not perf-warn on contiguous row-major input - #347

fix(lu): do not perf-warn on contiguous row-major input#347
shinaoka merged 4 commits into
mainfrom
345-lu-partial-perf-warn-row-major

shinaoka commented Nov 9, 2026 •

edited

Loading

Uh oh!

shinaoka commented Nov 9, 2026

Uh oh!

shinaoka commented Nov 9, 2026

Uh oh!

shinaoka commented Nov 9, 2026

Uh oh!

shinaoka commented Nov 9, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

shinaoka commented Nov 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The defect

The change

Verification

Red: the new test against the old predicate, moved into the helper verbatim

Green: the same test file, helper body fixed

Red: the unsigned_abs mutant

Green: the same test with the helper restored

Red: the call site, with the predicate untouched

Green: the call site restored

cargo test --lib lu::partial_pivoting, full output

Formatting

Clippy

Upstream

Not measured

CI

Uh oh!

shinaoka commented Nov 9, 2026

GPT-6.1-sol adversarial review

Critical (must not merge)

Needs changes

Unverified claims

Checked and fine

Not checked: repository code outside the supplied diff, Cargo execution, or live CI; runtime and performance claims remain unverified.

Uh oh!

shinaoka commented Nov 9, 2026

GPT-6.1-sol adversarial review

Critical (must not merge)

Needs changes

Unverified claims

Checked and fine

Not checked: compilation, runtime logging, live CI, or source outside the supplied diff; the test-wiring finding is a source-level mutation analysis, not an executed mutation.

Uh oh!

shinaoka commented Nov 9, 2026

GPT-6.1-sol adversarial review

Critical (must not merge)

Needs changes

Unverified claims

Checked and fine

Not checked: local compilation, test execution, mutation runs, or the final completion of pending CI. Review evidence is limited to the supplied diff and retrieved head files/check statuses.

Uh oh!

shinaoka commented Nov 9, 2026

GPT-6.1-sol adversarial review

Critical (must not merge)

Needs changes

Unverified claims

Checked and fine

Not checked: compilation, runtime mutation tests, or source outside the supplied diff; remaining risk is unverified execution evidence for the submitted head.

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

shinaoka commented Nov 9, 2026 •

edited

Loading

Red: the `unsigned_abs` mutant

`cargo test --lib lu::partial_pivoting`, full output